-
Notifications
You must be signed in to change notification settings - Fork 517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure constants written correctly to LP/NL files #2953
Conversation
@@ -1478,10 +1499,28 @@ def _write_nl_expression(self, repn, include_const): | |||
if include_const and repn.const: | |||
# Add the constant to the NL expression. AMPL adds the | |||
# constant as the second argument, so we will too. | |||
nl = self.template.binary_sum + nl + (self.template.const % repn.const) | |||
nl = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate what black
did to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had some thoughts about possible refactor opportunities, but they can be tabled.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2953 +/- ##
==========================================
+ Coverage 88.04% 88.10% +0.05%
==========================================
Files 768 769 +1
Lines 89260 89708 +448
==========================================
+ Hits 78592 79034 +442
- Misses 10668 10674 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Fixes #2927, Fixes #2928
Summary/Motivation:
The LP and NL writers were more flexible in the data types they accepted, but introduced cases where non-int/float data types were written out to the NL or LP files. This PR goes through and ensures that all numeric constants are mapped to either int or float being written out to the file.
Note that this restores the original behavior reported in the issue, but does not fix the slightly more egregious error of permitting the use of
Boolean
as a domain for (numeric)Constraint
s.Changes proposed in this PR:
int
orfloat
before writing them out to the LP/NL fileVar
andConstraint
lb/
ubproperties to ensure that the returned vales are either native numeric types or
None. This also ensures that ``inf
/-inf
are mapped to None before returning them to the userLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: